feat: allow use of OpenSCAP result files in task xccdf_result_to_oscal_ar #1411

Mab879 · 2023-06-27T18:54:07Z

Types of changes

Hot fix (emergency fix and release)
Bug fix (non-breaking change which fixes an issue)
[x ] New feature (non-breaking change which adds functionality)
Documentation (change which affects the documentation site)
Breaking change (fix or feature that would cause existing functionality to change)
Release (develop -> main)

Quality assurance (all should be covered).

My code follows the code style of this project.
Documentation for my change is up to date?
My PR meets testing requirements.
All new and existing tests passed.
All commits are signed-off.

Summary

Before this commit if you wanted to use result files from OpenSCAP in the task xccdf_result_to_oscal_ar you had to extract the TestResult element and place it as the root of the XML document, otherwise the resulting OSCAL document would be blank. Thus making it impossible to directly use output from OpenSCAP with the task.

With this commit the task will detect that the root element is not TestResult and then it will find the TestResult element in the XML document. This allows the use of files created by OpenSCAP using the --results and --results-arf switches.

Fixes #1410

Key links:

Sonar coverage

Before you merge

Ensure it is a 'squash commit' if not a release.
Ensure CI is currently passing
Check sonar. If you are working for a fork a maintainer will reach out, if required.

degenaro

Thanks for this submission!

Are other namespaces possible? If so should there be a list used until the right one is found?
Alternatively, should the trestle task xccdf-result-to-oscal-ar be enhanced to accept an optional config flag, perhaps namespace = http://checklists.nist.gov/xccdf/1.2?
Can you add a test case to tests/trestle/tasks/xccdf_result_to_oscal_ar_test.py?

Mab879 · 2023-06-30T01:37:11Z

1. Are other namespaces possible? If so should there be a list used until the right one is found?

There might be one more, I can add that to the task logic.

2. Alternatively, should the `trestle task xccdf-result-to-oscal-ar` be enhanced to accept an optional config flag, perhaps `namespace = http://checklists.nist.gov/xccdf/1.2`?

I think searching for the correct namespace might be best option.

3. Can you add a test case to `tests/trestle/tasks/xccdf_result_to_oscal_ar_test.py`?

Done. The test files are rather large, I welcome suggestions on this.

degenaro · 2023-06-30T13:24:19Z

With respect to the code: is http://checklists.nist.gov/xccdf/1.2 the only possibility or are there other common versions that should be checked if 1.2 is not the correct one (e.g. 1.1, 1.3, 2.0...)?

With respect to the test cases, please consider: it would seem that only 1 new test case and 1 corresponding data file are needed to cover the new code, and the test data file should be reduced (hacked, if you will) to the bare minimum amount needed to perform the new ns test.

Thx!

degenaro · 2023-11-16T19:04:43Z

Do you wish to continue with this PR? Another release of trestle is imminent, then the direction will be toward upgrading to the current version of OSCAL.

Also, the value for ns should not be hardcoded? Nor should the search term .//checklist12:TestResult ?

Mab879 · 2023-11-17T14:00:10Z

This is something that I'm still interested in working on.

The ns value is the XML namespace mapping and checklist12:TestResult is just using those XML namespaces.

degenaro · 2023-11-17T15:24:57Z

This is something that I'm still interested in working on.

The ns value is the XML namespace mapping and checklist12:TestResult is just using those XML namespaces.

But it is bad practice to hardocde values in code that are not typically universal, no?

Mab879 · 2023-11-30T22:22:05Z

This is something that I'm still interested in working on.
The ns value is the XML namespace mapping and checklist12:TestResult is just using those XML namespaces.

But it is bad practice to hardocde values in code that are not typically universal, no?

So the ns values are from the standards so the should be universal.

degenaro · 2023-12-05T13:00:04Z

Please try running:

make code-format
make code-lint

degenaro · 2023-12-07T17:02:59Z

We are close to making a release in the next few days. If you can run the make commands above and deliver the revised formatted code, I can approve, then you can merge. Otherwise there will be a freeze on new features until after upgrading to new version of OSCAL 1.1.2. Thx!

degenaro · 2023-12-07T18:15:17Z

tests/trestle/tasks/xccdf_result_to_oscal_ar_test.py:449:1: D103 Missing docstring in public function
tests/trestle/tasks/xccdf_result_to_oscal_ar_test.py:470:1: D103 Missing docstring in public function

Mab879 · 2023-12-07T18:35:58Z

tests/trestle/tasks/xccdf_result_to_oscal_ar_test.py:449:1: D103 Missing docstring in public function tests/trestle/tasks/xccdf_result_to_oscal_ar_test.py:470:1: D103 Missing docstring in public function

Thanks for the output. I have struggling to get make code-lint to run locally.

degenaro · 2023-12-07T20:26:23Z

Here is the trick:

python -m venv venv.trestle
source venv.trestle/bin/activate
cd compliance-trestle
make install
make develop
make code-format
make code-lint

degenaro

LGTM.

Please:

Update Branch (out of sync)
Squash and Merge

…l_ar Before this commit if you wanted to use result files from OpenSCAP in the task xccdf_result_to_oscal_ar you had to extract the `TestResult` element and place it as the root of the XML document, otherwise the resulting OSCAL document would be blank. Thus making it impossible to directly use output from OpenSCAP with the task. With this commit the task will detect that the root element is not `TestResult` and then it will find the `TestResult` element in the XML document. This allows the use of files created by OpenSCAP using the `--results` and `--results-arf` switches. Signed-off-by: Matthew Burket <[email protected]>

…r_test Signed-off-by: Matthew Burket <[email protected]>

Mab879 · 2023-12-07T21:05:09Z

LGTM.

Please:

1. Update Branch (out of sync)

2. Squash and Merge

Done. Looks like I will need approval again for the CI to run.

degenaro · 2023-12-11T13:25:52Z

I think you should be able to squash and merge now...?

Mab879 · 2023-12-11T14:34:34Z

I think you should be able to squash and merge now...?

I'm still seeing "Only those with write access to this repository can merge pull requests."

degenaro · 2023-12-11T15:08:32Z

Ugh. I did the squash and merge, no worries. Thanks for this contribution!

degenaro changed the title ~~feat: Allow use of OpenSCAP result files in task xccdf_result_to_oscal_ar~~ feat: allow use of OpenSCAP result files in task xccdf_result_to_oscal_ar Jun 28, 2023

degenaro reviewed Jun 28, 2023

View reviewed changes

Mab879 force-pushed the openscap_ar branch from 460b026 to b891177 Compare June 30, 2023 01:35

Mab879 force-pushed the openscap_ar branch from b292833 to 645c425 Compare November 17, 2023 14:07

Mab879 force-pushed the openscap_ar branch from 645c425 to ff6c013 Compare November 30, 2023 23:27

Mab879 marked this pull request as ready for review November 30, 2023 23:28

Mab879 force-pushed the openscap_ar branch from ff6c013 to 1244999 Compare December 7, 2023 17:26

Mab879 force-pushed the openscap_ar branch from 1244999 to 796034d Compare December 7, 2023 18:34

degenaro self-requested a review December 7, 2023 20:53

degenaro approved these changes Dec 7, 2023

View reviewed changes

Mab879 added 2 commits December 7, 2023 15:01

Add tests for OpenSCAP results files for task xccdf_result_to_oscal_a…

c7d7a95

…r_test Signed-off-by: Matthew Burket <[email protected]>

Mab879 force-pushed the openscap_ar branch from 796034d to c7d7a95 Compare December 7, 2023 21:01

degenaro merged commit eeb715c into oscal-compass:develop Dec 11, 2023
16 checks passed

Mab879 deleted the openscap_ar branch December 11, 2023 16:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: allow use of OpenSCAP result files in task xccdf_result_to_oscal_ar #1411

feat: allow use of OpenSCAP result files in task xccdf_result_to_oscal_ar #1411

Mab879 commented Jun 27, 2023

degenaro left a comment

Mab879 commented Jun 30, 2023

degenaro commented Jun 30, 2023

degenaro commented Nov 16, 2023

Mab879 commented Nov 17, 2023

degenaro commented Nov 17, 2023

Mab879 commented Nov 30, 2023

degenaro commented Dec 5, 2023

degenaro commented Dec 7, 2023

degenaro commented Dec 7, 2023

Mab879 commented Dec 7, 2023

degenaro commented Dec 7, 2023

degenaro left a comment

Mab879 commented Dec 7, 2023

degenaro commented Dec 11, 2023

Mab879 commented Dec 11, 2023

degenaro commented Dec 11, 2023

feat: allow use of OpenSCAP result files in task xccdf_result_to_oscal_ar #1411

feat: allow use of OpenSCAP result files in task xccdf_result_to_oscal_ar #1411

Conversation

Mab879 commented Jun 27, 2023

Types of changes

Quality assurance (all should be covered).

Summary

Key links:

Before you merge

degenaro left a comment

Choose a reason for hiding this comment

Mab879 commented Jun 30, 2023

degenaro commented Jun 30, 2023

degenaro commented Nov 16, 2023

Mab879 commented Nov 17, 2023

degenaro commented Nov 17, 2023

Mab879 commented Nov 30, 2023

degenaro commented Dec 5, 2023

degenaro commented Dec 7, 2023

degenaro commented Dec 7, 2023

Mab879 commented Dec 7, 2023

degenaro commented Dec 7, 2023

degenaro left a comment

Choose a reason for hiding this comment

Mab879 commented Dec 7, 2023

degenaro commented Dec 11, 2023

Mab879 commented Dec 11, 2023

degenaro commented Dec 11, 2023